Implementation Notes

Architecture

This PR leverages the existing S3 state-sync architecture. The init container (hydrate.sh) already downloads file-uploads/ from s3://{bucket}/{namespace}/{sessionName}/file-uploads/. By writing files to this same S3 path before the pod starts, they are naturally seeded into the workspace during initialization.

New Backend Endpoints

• PUT /projects/:name/agentic-sessions/:sessionName/file-uploads/*path — Upload file to S3
• GET /projects/:name/agentic-sessions/:sessionName/file-uploads — List pre-uploaded files
• DELETE /projects/:name/agentic-sessions/:sessionName/file-uploads/*path — Delete pre-uploaded file

All endpoints perform RBAC checks and path sanitization. The session CR must exist but the pod does not need to be running.

Frontend Flow

1. Create Session dialog: Users can attach files before creating the session. After session creation, files are uploaded to S3 via the new pre-upload endpoint.
2. Existing upload modal: When uploading to a non-running session (409 from workspace endpoint), the route automatically falls back to the pre-upload S3 endpoint.

Configuration

The backend reads S3 config from environment variables: S3_ENDPOINT, S3_BUCKET, AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY. If not configured, pre-upload endpoints return 503 (graceful degradation).

🤖 Session

@ambient-code

Pull Reviews

🚨 Concerning — Adds pre-upload file functionality allowing files to be staged in S3 before session startup, with frontend fallback handling, backend routing, S3 storage operations, and test coverage.

https://pub-510dcf6312d143b68159aa24908df5dc.r2.dev/videos/ci-ambient-code-platform-1282-1775846715016.mp4

Callouts

• critical (security): The fileUploadS3Key function constructs S3 paths using user-provided session names without validation. Malicious session names with path traversal characters (e.g., '../') could result in files being stored at unintended S3 locations. While individual file paths are validated with pathutil.IsPathWithinBase, the session name itself is not sanitized.
• warning (security): The payload size is not validated before uploading to S3. A malicious user could upload extremely large files, causing storage exhaustion or DoS. Consider implementing maximum file size limits and total quota per session.
• warning (testing): The file_uploads_test.go file only tests utility functions. Critical path logic including RBAC checks, S3 operations, and error handling is untested. Integration tests or mocked S3 tests are needed.
• warning (performance): The PreUploadFile handler reads the entire request body into memory with io.ReadAll before uploading to S3. For large files, this could consume excessive memory. Consider streaming directly to S3.
• info (complexity): Each of the three file operation handlers duplicates RBAC check logic, project/session parameter extraction, and error handling. Consider extracting these into middleware or helper functions to reduce duplication.
• critical (security): S3 credentials (AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY) are read directly from environment variables without validation. If these are exposed in logs or error messages, it could leak credentials. Consider masking in logs and validating credential format.
• warning (breaking-change): New endpoints are added to routes but corresponding handler functions (handlers.ListPreUploadedFiles, handlers.PreUploadFile, handlers.DeletePreUploadedFile) are not shown in the diff. If these handlers don't exist or have breaking signatures, the backend will fail to compile.
• warning (complexity): The pre-upload fallback uses HTTP 409 Conflict to signal session unavailable. This creates implicit contract between frontend and backend. If handlers don't return 409 in expected scenarios, fallback won't trigger. Consider explicit status code documentation.
• warning (performance): S3Client.ListObjects uses recursive listing without pagination limits. For buckets with many files under a prefix, this could load all objects into memory. Consider adding MaxKeys parameter or pagination support.
• info (testing): S3 tests only cover LoadS3ConfigFromEnv, not actual S3 operations (PutObject, ListObjects, DeleteObject, ObjectExists). Consider adding integration tests or mocked operation tests.
• info (testing): CreateSessionDialog file tests use complex mocking setup. The removeButtons filter logic is fragile and relies on specific class names and structure. Consider using data-testid attributes for more reliable selectors.
• warning (performance): Sequential file uploads in a for-loop after session creation could be slow with multiple files. No concurrent upload requests and no progress tracking per file, only overall completion state.
• warning (testing): New file upload logic added to onSuccess callback with no tests visible in this diff. The async upload flow with error handling needs test coverage for success/failure scenarios.
• info (security): Files are uploaded directly to S3 via the API endpoint. Ensure the backend endpoint validates file types, scans for malware, and enforces per-user/session storage quotas. File type validation is not performed on the frontend.
• info (complexity): Error handling for partial upload failures silently continues and shows a generic toast. Users won't know which specific files failed if some uploads succeed and others fail.

_{Generated by pull-reviews — automated video reviews for PRs}

✅ Deploy Preview for cheerful-kitten-f556a0 canceled.

⚠️ SDD Preflight — Managed Paths Modified

This PR modifies files in SDD-managed component(s). These components are migrating to Spec-Driven Development.

No action required — these components are in warn mode. Consider using the component's agent workflow for future changes.

📖 Specs: Runner Spec · Runner Constitution